Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use guide-key with window-purpose [fixes #915] #1072

Closed
wants to merge 1 commit into from
Closed

Use guide-key with window-purpose [fixes #915] #1072

wants to merge 1 commit into from

Conversation

bmag
Copy link
Contributor

@bmag bmag commented Apr 4, 2015

Add window-purpose package, make guide-key use it as a backend instead
of popwin. When window-purpose can't display guide-key buffer (e.g. if
there are too many open windows), fallback to popwin.
This fixes bug of guide-key buffer being too small when there are
several windows open (issue #915).

Add window-purpose package, make guide-key use it as a backend instead
of popwin.  When window-purpose can't display guide-key buffer (e.g. if
there are too many open windows), fallback to popwin.
This fixes bug of guide-key buffer being too small when there are
several windows open (issue #915).
@bmag
Copy link
Contributor Author

bmag commented Apr 4, 2015

This should work, but maybe someone else should also test that it works for him/her.
If the names or the location of the functions is wrong, tell me and I'll move/rename them.

@syl20bnr
Copy link
Owner

syl20bnr commented Apr 5, 2015

Outch that's a lot of code for this :-)
I'm ok with this but I have a couple of questions:

  • What is the original behavior of guide-key (how does it display its window) ?
  • Is there already a variable in guide-key to specifiy a minumum size ?
  • Using only popwin is not enough ? Why ?
  • What purpose-window brings to the table regarding windows size ?

I know this is a lot of questions but I want to understand your fix :-)

@bmag
Copy link
Contributor Author

bmag commented Apr 5, 2015

No problem, it's good that you have questions.

Just to be clear, window-purpose can do other stuff as well, but they're irrelevant for this fix. I'm only using some of its functionality here. Mainly purpose-display-at-right (or left/top/bottom) to create a window for guide-key (in function spacemacs/display-guide-key).

Anyway, here are the answers for your questions:

  • Originally, guide-key uses popwin:popup-buffer for displaying the guide-key buffer and popwin:close-popup-window for closing it. It calls popwin:popup-buffer with parameters :position guide-key/popup-window-position and :height (guide-key/popup-window-size) or :width (guide-key/popup-window-size 'horizontal).
  • What do you mean by "a minimum size"? guide-key calculates how much size it needs via guide-key/popup-window-size function. Please explain.
  • Using popwin works fine most of the time, but when the frame is split into too many windows, popwin creates a window that is too small for the guide-key buffer (see issue guide-key window is sized incorrectly when other windows don't span the frame #915). If you think we can go on without fixing this issue, then we don't this PR.
  • What window-purpose brings to the table, in regarding guide-key's window size, is that window-purpose can create a big enough window in cases where popwin can't. If you split your frame with SPC w / a few times, you'll get to a point where guide-key's window isn't big enough to show all the available keys.
    With window-purpose, you won't encounter this problem, unless there really isn't enough space in the frame.
    To illustrate, using popwin, this is the frame after I press space:
    guide-key-with-popwin
    And using window-purpose, this is the frame after I press space:
    guide-key-with-purpose

Of course, there may be a way fix #915 without replacing popwin, but since you said this is a recurring issue with popup buffers, I propose to replace popwin with window-purpose, for guide-key.

@bmag
Copy link
Contributor Author

bmag commented Apr 18, 2015

@syl20bnr are you still considering this PR? If you decided to reject it, tell me so I can delete it.

@trishume
Copy link
Contributor

My only concern is that larger guide-key windows could be annoying. I think ideally the behaviour would be to after a short time pop up a narrow window and then have it widen after a delay or keybind.

Constantly having windows be blocked by massive guide-key popups could be annoying for some slower typists.

@tuhdo
Copy link
Contributor

tuhdo commented Apr 19, 2015

@syl20bnr Let me briefly explain about window-purpose:as its name implies, it gives each window a purpose and only assigns buffers that have the same purpose to a window. For example, if a window is only used for displayed Emacs Lisp source files, then only Emacs LIsp buffers are used for that buffer when you switch buffer or open file. Buffers that are not the same purpose with current window, are displayed in other window that has the same purpose, or create a new one if one does not exist.

Let's look at a more complicated example: you have 3 windows, one to the left and two to the right. The left window purpose is for Emacs Lisp buffer, the top right is for shell buffer and the bottom right is general purpose. In that case, whenever you open a new general purpose buffer like a help buffer or a diff buffer or a compilation buffer, it always displays at the bottom right.

This mechanism is more sophisticated than the stock other window mechnism of stock Emacs that opens in any window it sees fits. Personally, I prefer this mechanism over popwin. Currently, pop-win is messing up all the REPL buffers of SLIME (i.e. the inspector buffer). We won't want to fix this issue whenever we want to add a new layer that opens window here and there. Spacemacs also opens buffer like *diff* or *help* at the bottom window, making the buffers hard to use. With window-purpose, no purpose buffers like that are opened in new windows and not our existing one.

@tuhdo
Copy link
Contributor

tuhdo commented Apr 19, 2015

@syl20bnr you can also have a look at this reddit thread for visual demonstration.

@trishume
Copy link
Contributor

Whoah cool @tuhdo thanks! There should be a layer for this. I am often annoyed by random help and magit buffers wrecking all my windows.

Although what you are describing I don't think is a good fit for guide-key. guide-key I think should always be a new window on the side that dissapears, it shouldn't take over even a no-purpose window. But I think this might be a matter of personal preference. And as I think this PR shows window-purpose can be tuned to do exactly that, with 60 lines of additional code.

@tuhdo
Copy link
Contributor

tuhdo commented Apr 19, 2015

@trishume It's the default behaviour of guide-key that pop in and out automatically and is not related to pop-win, so we can safely disabled pop-win. I think we should make pop-win a layer since it currently is messing with windows of major modes that create its windows.

@syl20bnr
Copy link
Owner

@bmag honestly I find the code very complicated for such a tweak, I feel that it should be something coded upstream in guide-key.

BUT window-purpose as a great role to play in spacemacs, I'm not entirely sure for now but it could be as important as helm, use-package, projectile and the like. Let's find some usage pattern where window-purpose shines. The obvious first one I see is the REPL case, having the REPL always appearing by default in the same place is very appealing to me.

Is it easy to move around the purposed window ? Say that the user wants the REPL below temporarily because it has a wide output.

@tuhdo
Copy link
Contributor

tuhdo commented Apr 21, 2015

@syl20bnr we can combine window-purpose to the proposed features in this issue: #1222. That way, window-purpose can display buffer in far away window and users can delete any window on screen without moving point anywhere. That way, we won't need popwin anymore.

@bmag
Copy link
Contributor Author

bmag commented Apr 21, 2015

@syl20bnr here are some answers. It turned out to be a lengthy response, sorry about that.

Admittedly this PR I proposed is a bit complicated for the issue it fixes. I mainly opened the PR so you can review and decide if you like it (which is how PRs are supposed to work, isn't it?). I'll check if guide-key's author thinks using window-purpose is a good idea.

About use-cases for window-purpose, I think (and hope) it can do a lot of things with proper configuration and extensions. The main thing that window-purpose does is to maintain a robust window layout. That means that buffers are displayed in proper windows, as @tuhdo explained.

Another thing you can do is quickly open a stored window layout (purpose-load-window-layout) - saves you the effort of manually setting the layout everytime. By default only restores the position, purpose and purpose-dedicated state of each window, but can be extended with hooks.

Other than that, you can extend window-purpose to support other use-cases: (I tried to make it easy to extend)

  • The REPL use-case you mentioned. Configure all relevant buffers (according to major-mode/buffer name) to have a 'repl purpose, and add an entry for 'repl in purpose-special-action-sequences. It's easier than it sounds.
  • Open some buffers in a seperate frame. For example, you can have Magit buffers to always be opened in their own frame, and close that frame when you finished commiting/viewing diffs/whatever. I used to open the package-list buffer in a separate frame before converting to Spacemacs.
  • Combine with perspective, so some buffers have a different purpose depending on the current perspective. Also possible to load some layout when opening some perspective.
  • Emulate popwin. The tricky part, which I haven't looked into yet, is how to automatically close the popup-window the way popwin does it.
  • Have a right, left, top and bottom "panes". Sort of what IDEs have, that some buffers open at specific sides, and you can "pin" them or let them "auto-hide". This could be a bit tricky with window-purpose, but still doable with purpose-special-action-sequences.

And I'm sure more things can be done. In window-purpose I try to provide a good framework for extending Emacs' window behavior.

About moving a window around, there isn't an easy way to do it with window-purpose. You could close the REPL window, create and select a new window at the bottom, then use purpose-set-window-purpose or switch-buffer-without-purpose to display the REPL in the new window. Or you can write a function based on this:

(defun open-current-buffer-at-bottom ()
  (interactive)
  (select-window (purpose-display-at-bottom (current-buffer) nil)))

There is one problem that you should know about, though. Right now there is a problem with using window-purpose and popwin together. When popwin opens a popup-window, it removes the purpose-dedicated state of all windows. I opened a (very simple) PR against popwin that fixes this problem three weeks ago, but didn't receive a response yet.

@tuhdo I'm not sure I understood what you said about issue #1222. With window-purpose, when you open a buffer it should automatically open in the correct window, so you don't need SPC b <number> for that. But you could use SPC b <number> to ignore window-purpose and force some buffer to open in a certain window.

@tuhdo
Copy link
Contributor

tuhdo commented Apr 21, 2015

@bmag yes, if window-purpose is merged, it is not needed anymore because window-purpose can automate this. However, deleting window without moving point is still needed.

@sooheon
Copy link

sooheon commented Aug 12, 2015

Window purpose is a great layer, why was this PR closed? I thought it was still under discussion? Spacemacs still has a pain point with window popping and buffer management (ex. inconsistency in whether certain buffers pop up with focus or not, which buffers SPC-tab sometimes taking you to helm buffers, etc.).

@syl20bnr
Copy link
Owner

@sooheon There is another PR for this. I'm aware that spacemacs needs a good solution for layout consistency and Windows Purpose is well placed to fill the gap.

@syl20bnr
Copy link
Owner

@sooheon here it is: #1958

All I can say is, be patient ;-)

@sooheon
Copy link

sooheon commented Aug 12, 2015

Ah, I made a search and entered the wrong thread. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants